Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Realtime dictation drops the existing draft — the composer is cleared before
stopAndSend, but the realtime implementation receives noinitialTextand defaults it to an empty string. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:486,web/src/hooks/useDictation.ts:190,web/src/hooks/useRealtimeDictation.ts:242.
Suggested fix:const initialText = api.composer().getState().text api.composer().setText("") await dictation.stopAndSend?.(targetSessionId, initialText)
- [Major] Stopping during
connectingclears the draft without sending it — both hooks cancel startup when no recorder/session exists, so no final callback consumes the pending send. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:484with existing stop branches atweb/src/hooks/useDictation.ts:156andweb/src/hooks/useRealtimeDictation.ts:220.
Suggested fix:if (dictation.status === "connecting") { await dictation.toggle() return // cancel startup; preserve composer text } if (dictation.status === "connected") { // stopAndSend path }
- [Major] Voice finalization bypasses the normal composer payload —
stopAndSendsends only(sessionId, text), silently omitting attachments, pending schedules, scratchlist routing, and delivery intent even though dictation can start with those states present. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:488,web/src/hooks/useDictation.ts:126,web/src/hooks/useRealtimeDictation.ts:65; attachment-backedcanSendcontext atweb/src/components/AssistantChat/HappyComposer.tsx:668.
Suggested fix:if (attachments.length > 0 || pendingSchedule != null || props.scratchlistMode) { await dictation.toggle() return // retain the full payload for the normal send path } await dictation.stopAndSend?.(targetSessionId)
- [Major] Cross-session voice-tool delivery is fire-and-forget — the newly returned
api.sendMessagepromise is ignored, while the tool immediately reportssent; HTTP failures become unhandled rejections and the voice agent receives false success. Evidenceweb/src/components/SessionChat.tsx:1113, related call atweb/src/realtime/realtimeClientTools.ts:53.
Suggested fix:// session-store contract: sendMessage returns Promise<void> sendMessage: async (sessionId, message) => { if (sessionId === props.session.id) { const accepted = await props.onSend(message) if (!accepted) throw new Error("Message was not accepted") } else { await props.api.sendMessage(sessionId, message) } } // realtimeClientTools await sessionStore.sendMessage(sessionId, message)
Questions
- None.
Summary
Review mode: initial
Four Major correctness/data-loss issues found in the new background voice-send paths. Existing added tests do not cover realtime draft preservation, cancellation while connecting, complex composer payloads, or request rejection.
Testing
- Not run (automation): untrusted PR code was not executed per review security requirements. GitHub checks were pending when reviewed.
HAPI Bot
| const targetSessionId = props.sessionId ?? '' | ||
| api.composer().setText('') | ||
| if (targetSessionId) { | ||
| await dictation.stopAndSend?.(targetSessionId) |
There was a problem hiding this comment.
[MAJOR] Realtime mode loses the pre-existing composer draft. This call passes only the session id after clearing the composer. Standard mode snapshots getCurrentText(), but useDictation forwards the realtime stopAndSend unchanged and that hook defaults initialText to ""; therefore draft + spoken words becomes only spoken words in realtime mode.
Suggested fix:
const initialText = api.composer().getState().text
api.composer().setText("")
await dictation.stopAndSend?.(targetSessionId, initialText)Update both stop-and-send implementations to accept the explicit snapshot, and add a realtime regression test with an existing draft.
| const dictationActive = voiceInput.voiceMode === 'dictation' | ||
| const effectiveVoiceStatus = dictationActive ? dictation.status : voiceStatus | ||
| const handleDictationToggle = useCallback(async () => { | ||
| if (dictationActive && (dictation.status === 'connected' || dictation.status === 'connecting')) { |
There was a problem hiding this comment.
[MAJOR] The connecting branch clears text but cannot complete a send. During microphone permission/token startup, neither hook has a recorder/session yet. Their stop path only aborts startup and sets disconnected; no onstop/finish callback runs, so the pending send remains unused and the cleared draft is lost.
Suggested fix:
if (dictation.status === "connecting") {
await dictation.toggle()
return // cancel startup without clearing the draft
}
if (dictation.status === "connected") {
// snapshot, clear, and stopAndSend
}| const handleDictationToggle = useCallback(async () => { | ||
| if (dictationActive && (dictation.status === 'connected' || dictation.status === 'connecting')) { | ||
| const targetSessionId = props.sessionId ?? '' | ||
| api.composer().setText('') |
There was a problem hiding this comment.
[MAJOR] This text-only finalizer bypasses the composer send contract. Dictation is available when attachments or a pending schedule exist, but stopAndSend ultimately calls api.sendMessage(sessionId, finalMessage) without attachments, scheduling, scratchlist routing, or queue intent. The text is cleared while the rest of the payload remains, so pressing the active dictation button can send an incomplete/immediate message.
Suggested fix: snapshot and route the finalized text through the existing session-bound send mutation. As a minimal safety guard until that payload is supported:
if (attachments.length > 0 || pendingSchedule != null || props.scratchlistMode) {
await dictation.toggle()
return
}
await dictation.stopAndSend?.(targetSessionId)| if (sessionId === props.session.id) { | ||
| props.onSend(message) | ||
| } else { | ||
| props.api.sendMessage(sessionId, message) |
There was a problem hiding this comment.
[MAJOR] Await the cross-session HTTP send before reporting success. This new branch returns a Promise<void>, but the session-store contract is void and messageCodingAgent does not await it. A rejected request therefore becomes an unhandled rejection while the tool returns sent.
Suggested fix:
// session-store contract
sendMessage: (sessionId: string, message: string) => Promise<void>
sendMessage: async (sessionId, message) => {
if (sessionId === props.session.id) {
const accepted = await props.onSend(message)
if (!accepted) throw new Error("Message was not accepted")
} else {
await props.api.sendMessage(sessionId, message)
}
}
// realtimeClientTools
await sessionStore.sendMessage(sessionId, message)
There was a problem hiding this comment.
Findings
- [Major] Realtime stop-and-send is canceled by route unmount —
stopAndSendwaits for provider finalization, but cleanup increments the operation and cancels the session beforefinish()can consume the pending send. Evidenceweb/src/hooks/useRealtimeDictation.ts:243, with teardown context atweb/src/hooks/useRealtimeDictation.ts:251.
Suggested fix:return () => { mountedRef.current = false if (sendOnFinishRef.current) return operationRef.current += 1 sessionRef.current?.cancel() }
- [Major] Inactive-session dictation takes a send path the hub always rejects — the composer allows voice input on inactive sessions, but the new direct
api.sendMessagepath bypasses the normal resume resolver; the hub message route requires an active session and returns 409. Because the composer is cleared first, this loses the draft. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:490, related hub contracthub/src/web/routes/messages.ts:94.
Suggested fix:if (!props.active || attachments.length > 0 || props.pendingSchedule != null || props.scratchlistMode) { await dictation.toggle() return }
- [Major] Background send failures discard the cleared draft — standard mode catches failures without restoring text, while realtime mode uses an unhandled fire-and-forget promise. Navigation makes the failure silent and leaves no retry surface. Evidence
web/src/hooks/useRealtimeDictation.ts:65, related standard pathweb/src/hooks/useDictation.ts:126and clear atweb/src/components/AssistantChat/HappyComposer.tsx:496.
Suggested fix:try { await config.api.sendMessage(pendingSend.sessionId, finalMessage) } catch (error) { updateComposerDraftTextSnapshot(pendingSend.sessionId, finalMessage) if (mountedRef.current) config.onTextChange(finalMessage) throw error }
- [Major] Dictation finalization bypasses rich-composer serialization — both the early send branch and mic-toggle snapshot run before the existing
flushSerializedText()contract, so the final message can omit a pending rich edit/session mention. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:1130, related snapshot atweb/src/components/AssistantChat/HappyComposer.tsx:495.
Suggested fix:const initialText = richMentionsEnabled && richInputRef.current ? richInputRef.current.flushSerializedText() : api.composer().getState().text
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Four Major correctness/data-loss issues remain in realtime teardown, inactive-session delivery, failure recovery, and rich-composer serialization.
Testing
- Not run (automation): untrusted PR code was not executed. Missing coverage: realtime
stopAndSendfollowed by unmount, inactive-session dictation, rejected transcription/send restoration, and rich-mention dictation finalization.
HAPI Bot
|
|
||
| const stopAndSend = useCallback(async (targetSessionId: string, initialText?: string) => { | ||
| sendOnFinishRef.current = { sessionId: targetSessionId, initialText: initialText ?? '' } | ||
| await stop() |
There was a problem hiding this comment.
[MAJOR] Keep pending realtime finalization alive across unmount. stopAndSend awaits session.stop(), but navigating to another session runs the cleanup at lines 251-263: it increments operationRef and cancels the provider session. OpenAI/Deepgram may wait up to 2.5 seconds for the final transcript; their guarded onFinal then cannot call finish(), so sendOnFinishRef is never consumed and the message is dropped. ElevenLabs has the same outcome because cleanup disables the active flag before its commit completes. The added unmount test covers standard MediaRecorder only.
Suggested fix:
return () => {
mountedRef.current = false
if (sendOnFinishRef.current) return // session.stop() owns finalization and cleanup
operationRef.current += 1
startAbortRef.current?.abort()
sessionRef.current?.cancel()
}| return | ||
| } | ||
| if (dictation.status === 'connected') { | ||
| if (attachments.length > 0 || props.pendingSchedule != null || props.scratchlistMode) { |
There was a problem hiding this comment.
[MAJOR] Guard inactive sessions before selecting the direct-send path. HappyComposer is mounted with allowSendWhenInactive and always receives voiceTranscriptionApi, so dictation remains available on an inactive session. This guard does not check props.active; stopping then clears the composer and the hook calls raw api.sendMessage. The hub POST route uses requireActive: true (hub/src/web/routes/messages.ts:94), so this path deterministically returns 409 instead of using the normal useSendMessage resume flow.
Suggested fix:
if (!props.active || attachments.length > 0 || props.pendingSchedule != null || props.scratchlistMode) {
await dictation.toggle() // append transcript; normal send can resume the session
return
}| if (pendingSend) { | ||
| const finalMessage = appendTranscript(pendingSend.initialText, text) | ||
| if (finalMessage.trim() && config.api) { | ||
| void config.api.sendMessage(pendingSend.sessionId, finalMessage) |
There was a problem hiding this comment.
[MAJOR] Preserve a retryable draft when the background send fails. The composer was already cleared at HappyComposer.tsx:496. Here the realtime request is fire-and-forget, so rejection becomes unhandled; the standard path awaits it but its catch only sets an error and also never restores the message. If the user navigated, even that error is suppressed. A transient 409/5xx/network failure therefore destroys the typed draft and dictated result.
Suggested fix (apply the same recovery to both hooks):
try {
await config.api.sendMessage(pendingSend.sessionId, finalMessage)
} catch (error) {
updateComposerDraftTextSnapshot(pendingSend.sessionId, finalMessage)
if (mountedRef.current) config.onTextChange(finalMessage)
throw error
}| }, [pendingSendIntentRef]) | ||
|
|
||
| const handleSend = useCallback(async (intent: ComposerSendIntent = 'default') => { | ||
| if (dictationActive && (dictation.status === 'connected' || dictation.status === 'connecting')) { |
There was a problem hiding this comment.
[MAJOR] Flush the rich editor before this early dictation-send branch. The existing flushSerializedText() block is below this return path at lines 1147-1151, and the mic button calls handleDictationToggle directly. That handler snapshots api.composer().getState().text, so a DOM-side rich edit/session mention that has not yet crossed the synchronous serialization boundary can be omitted from the sent message. The surrounding comment explicitly requires rich chips to be serialized before every send.
Suggested fix: move the dictation handler below richInputRef initialization and snapshot through the imperative flush on both entry points:
const initialText = richMentionsEnabled && richInputRef.current
? richInputRef.current.flushSerializedText()
: api.composer().getState().text
3257d42 to
7a79c03
Compare
There was a problem hiding this comment.
Findings
- [Major] Failure recovery still drops the original draft — the composer is cleared before background finalization, but realtime restores only the transcript while mounted, and both hooks suppress recovery after navigation. A transient transcription/send failure can therefore lose the typed prefix or the entire message. Evidence
web/src/hooks/useRealtimeDictation.ts:68, relatedweb/src/hooks/useDictation.ts:129andweb/src/components/AssistantChat/HappyComposer.tsx:497.
Suggested fix:catch (sendError) { updateComposerDraftTextSnapshot(pendingSend.sessionId, finalMessage) if (mountedRef.current) { onFinalTranscriptRef.current(finalMessage) setError(sendError instanceof Error ? sendError.message : 'Failed to send message') setStatus('error') } return }
- [Major] Cross-session permission tools cannot load the voice target — after navigation the registered store only exposes the newly viewed session, while the voice session ID remains the original target.
processPermissionRequesttherefore receivesnulland refuses every approval/denial for that target. Evidenceweb/src/components/SessionChat.tsx:1108, relatedweb/src/realtime/realtimeClientTools.ts:91.
Suggested fix:// Store contract getSession: (sessionId: string) => Promise<{ agentState?: { requests?: Record<string, unknown> } } | null> | { agentState?: { requests?: Record<string, unknown> } } | null // Tool const session = await sessionStore.getSession(sessionId) // Registration getSession: async (sessionId) => sessionId === props.session.id ? props.session : (await props.api.getSession(sessionId)).session
Questions
- None.
Summary
Review mode: follow-up after new commits
- Two Major correctness/data-loss issues remain in background failure recovery and cross-session voice permission handling.
Testing
- Not run (automation): untrusted PR code was not executed. GitHub
testcheck was pending at review time. - Missing coverage: rejected realtime send after typed-prefix dictation, rejected standard/realtime send after unmount, transcription failure after unmount, and permission approval after navigating away from the voice target.
HAPI Bot
| try { | ||
| await config.api.sendMessage(pendingSend.sessionId, finalMessage) | ||
| } catch (sendError) { | ||
| if (mountedRef.current) { |
There was a problem hiding this comment.
[MAJOR] Preserve the full draft even after navigation or send failure. HappyComposer clears the composer before stopAndSend (HappyComposer.tsx:497). When this send fails while mounted, onFinalTranscriptRef.current(text) restores only the dictated suffix, so any typed prefix in pendingSend.initialText is lost. When the route has unmounted, this branch performs no recovery at all; the standard hook has the same mounted-only guard, so a transient network error discards the entire cleared message.
Suggested fix (and apply equivalent persistence in the standard hook, including transcription/no-audio failures):
catch (sendError) {
updateComposerDraftTextSnapshot(pendingSend.sessionId, finalMessage)
if (mountedRef.current) {
onFinalTranscriptRef.current(finalMessage)
setError(sendError instanceof Error ? sendError.message : 'Failed to send message')
setStatus('error')
}
return
}| approvePermission: async (_sessionId: string, requestId: string) => { | ||
| await props.api.approvePermission(props.session.id, requestId) | ||
| props.onRefresh() | ||
| getSession: (sessionId: string) => (sessionId === props.session.id ? (props.session as unknown as { agentState?: { requests?: Record<string, unknown> } }) : null), |
There was a problem hiding this comment.
[MAJOR] Keep permission lookup bound to the voice target across navigation. The global voice session keeps its original currentSessionId, but navigating re-registers this store with the newly viewed props.session. For the original target this function now always returns null, so processPermissionRequest exits with “no active permission request” before the newly target-aware approvePermission/denyPermission callbacks can run.
Suggested fix:
// Allow async lookup in realtimeClientTools, then await it.
const session = await sessionStore.getSession(sessionId)
// Register a lookup that can load the non-visible target.
getSession: async (sessionId) =>
sessionId === props.session.id
? props.session
: (await props.api.getSession(sessionId)).session
There was a problem hiding this comment.
Findings
- [Major] Pending dictation text is still lost on finalization failures —
HappyComposerclears the composer before finalization, but standard no-audio/transcription failures and both hooks’ post-navigation send failures only update mounted UI. The captured typed prefix therefore has no session-keyed recovery after navigation. Evidenceweb/src/hooks/useDictation.ts:144, relatedweb/src/components/AssistantChat/HappyComposer.tsx:497andweb/src/hooks/useRealtimeDictation.ts:67.
Suggested fix:const recoverPendingSend = (pending: PendingSend, text: string, error: unknown) => config.onPendingSendError(pending.sessionId, text, error) // Invoke for no-audio, transcription, stop, and send failures, // including after unmount; back this callback with session-keyed draft/error state. recoverPendingSend(pendingSend, finalMessage, sendError) return
- [Minor] Finalized dictation bypasses the composer delivery policy — both hooks call
ApiClient.sendMessagewithout adeliveryMode. The hub normalizes that toqueue, while a normal submission to a thinking Pi session resolves tosteer; dictation instructions therefore wait behind the active turn instead of steering it. Evidenceweb/src/hooks/useDictation.ts:127, relatedweb/src/hooks/useRealtimeDictation.ts:66,web/src/components/SessionChat.tsx:1607, andhub/src/sync/messageService.ts:89.
Suggested fix:// Inject from SessionChat; do not call ApiClient directly in the hook. await config.sendFinalizedMessage(pendingSend.sessionId, finalMessage) const deliveryMode = resolveMessageDeliveryMode({ agentFlavor, isSessionThinking: props.session.thinking, intent: 'default' }) const accepted = await props.onSend(text, undefined, null, deliveryMode) if (!accepted) throw new Error('Message was not accepted')
Summary
Review mode: follow-up after new commits
- The permission lookup issue from the previous bot review is addressed. Draft recovery remains incomplete, and finalized dictation changes Pi delivery semantics.
Testing
- Not run (automation): untrusted PR code was not executed. GitHub
testcheck passed. - Missing coverage: no-audio/transcription/stop failure after composer clear, send rejection after unmount for both modes, and dictation send while a Pi session is thinking.
HAPI Bot
| if (!mountedRef.current) return | ||
| setError(transcriptionError instanceof Error ? transcriptionError.message : 'Transcription failed') | ||
| setStatus('error') | ||
| if (mountedRef.current) { |
There was a problem hiding this comment.
[MAJOR] Preserve the pending message on every failure path. The pending send is detached from the composer at HappyComposer.tsx:497, but this catch only reports an error while mounted. After navigation it silently drops pendingSend.initialText plus the transcript; the no-audio and transcription-error branches above have the same loss, and realtime uses the same mounted-only recovery.
Suggested fix:
const recoverPendingSend = (pending: PendingSend, text: string, error: unknown) =>
config.onPendingSendError(pending.sessionId, text, error)
// Call this for no-audio, transcription, stop, and send failures,
// even when the hook has unmounted; persist/recover by session id.
recoverPendingSend(pendingSend, finalMessage, sendError)
return| const finalMessage = appendTranscript(pendingSend.initialText, transcribedText) | ||
| if (finalMessage.trim() && config.api) { | ||
| try { | ||
| await config.api.sendMessage(pendingSend.sessionId, finalMessage) |
There was a problem hiding this comment.
[MINOR] Keep normal Pi delivery semantics for finalized dictation. This direct API call omits deliveryMode, so the hub normalizes the message to queue. A normal composer submission uses resolveMessageDeliveryMode and sends steer while a Pi session is thinking; voice dictation now waits behind that turn instead.
Suggested fix:
// Inject a target-aware sender from SessionChat.
await config.sendFinalizedMessage(pendingSend.sessionId, finalMessage)
const deliveryMode = resolveMessageDeliveryMode({
agentFlavor, isSessionThinking: props.session.thinking, intent: 'default'
})
const accepted = await props.onSend(text, undefined, null, deliveryMode)
if (!accepted) throw new Error('Message was not accepted')
There was a problem hiding this comment.
Findings
- [Major] Finalized dictation can overwrite a newer draft and send unrelated attachments -- transcription/send completes asynchronously while the composer remains editable; the callback later replaces live text and sends current attachments. Evidence
web/src/components/AssistantChat/HappyComposer.tsx:475, relatedweb/src/components/AssistantChat/HappyComposer.tsx:507andweb/src/lib/assistant-runtime.ts:759.
Suggested fix:// SessionChat: send the captured payload without mutating assistant-ui state. const accepted = await props.onSend(text, undefined, null, deliveryMode) if (!accepted) throw new Error("Message was not accepted")
- [Minor] Standard dictation ignores the selected language -- the request hard-codes
language: undefinedalthough settings persisthapi-voice-lang. Evidenceweb/src/hooks/useDictation.ts:134, relatedweb/src/routes/settings/useVoiceSettings.ts:98.
Suggested fix:const language = localStorage.getItem("hapi-voice-lang") || undefined // ... language
- [Minor] Standard dictation advertises support without recording APIs --
standardSupportedomitsbrowserCanRecordbut is returned to the composer. Evidenceweb/src/hooks/useDictation.ts:224, relatedweb/src/hooks/useDictation.ts:49.
Suggested fix:const standardSupported = config.mode === "standard" && config.api !== null && config.provider !== null && browserCanRecord
Summary
Review mode: follow-up after new commits
- Current head retains one data-loss/cross-payload risk and two standard-dictation regressions.
Testing
- Not run (automation): untrusted PR code was not executed. GitHub
testcheck passed. - Missing coverage: edits/attachments during pending finalization, standard-language forwarding, unsupported-browser capability reporting.
HAPI Bot
| onTextChange: setComposerText, | ||
| sendMessage: async (sessionId: string, text: string) => { | ||
| if (sessionId === props.sessionId) { | ||
| api.composer().setText(text) |
There was a problem hiding this comment.
[MAJOR] Finalized dictation can overwrite newer composer state. After line 507 clears the original draft, transcription continues asynchronously while input and attachment controls remain enabled. This setText replaces anything typed meanwhile, and composer().send() consumes attachments added during that window.
Suggested fix:
// Inject a SessionChat callback that sends the captured payload directly.
const accepted = await props.onSend(text, undefined, null, deliveryMode)
if (!accepted) throw new Error("Message was not accepted")Do not write the finalized payload through the mutable assistant-ui composer.
| file: new File([blob], `speech.${recordingExtension(type)}`, { type }), | ||
| provider: config.provider!, | ||
| mode: 'standard', | ||
| language: undefined |
There was a problem hiding this comment.
[MINOR] Preserve the configured language for standard transcription. Voice settings store hapi-voice-lang, and this hook forwarded it before the refactor. Hard-coding undefined makes standard transcription ignore that preference.
Suggested fix:
const language = localStorage.getItem("hapi-voice-lang") || undefined
// ...
language| ? realtime | ||
| : { supported: standardSupported, status, error, partialTranscript: '', toggle } | ||
| ? { ...realtime, stopAndSend: realtime.stopAndSend } | ||
| : { supported: standardSupported, status, error, partialTranscript: '', toggle, stopAndSend } |
There was a problem hiding this comment.
[MINOR] Return the actual browser capability. standardSupported no longer includes browserCanRecord, so browsers without getUserMedia or MediaRecorder expose an enabled dictation button that only fails after click.
Suggested fix:
const standardSupported = config.mode === "standard"
&& config.api !== null
&& config.provider !== null
&& browserCanRecord
There was a problem hiding this comment.
Findings
- [Major] Failed background dictation can clobber a replacement draft — after the captured draft is cleared, the composer remains editable while transcription and the API send run. If that send fails, the standard path unconditionally replaces the current text with the failed voice payload; the realtime path calls
onFinalTranscript, which appends that payload to whatever the user typed meanwhile. Evidenceweb/src/hooks/useDictation.ts:149, relatedweb/src/hooks/useRealtimeDictation.ts:73andweb/src/components/AssistantChat/HappyComposer.tsx:502.
Suggested fix:// Route the captured payload through SessionChat/useSendMessage so its // existing generation-guarded send-error recovery owns failure restoration. <HappyComposer onVoiceSend={(text) => props.onSend(text, undefined, null)} /> sendMessage: async (_sessionId, text) => { const accepted = await props.onVoiceSend?.(text) if (!accepted) throw new Error('Message was not accepted') }
Summary
Review mode: follow-up after new commits
- One Major draft-corruption risk remains on failed background voice sends.
Testing
- Not run (automation): untrusted PR code was not executed.
- Missing coverage: type a replacement draft while standard/realtime finalization is pending, then reject the send and verify the replacement is not overwritten or merged.
HAPI Bot
| } catch (sendError) { | ||
| saveDraft(pendingSend.sessionId, finalMessage) | ||
| if (mountedRef.current) { | ||
| config.onTextChange(finalMessage) |
There was a problem hiding this comment.
[MAJOR] Failed background dictation can overwrite a newer draft. After HappyComposer clears the captured text, input remains editable while transcription and this send run. If the request fails, this unconditional write replaces anything typed meanwhile; the realtime equivalent at useRealtimeDictation.ts:73 appends the failed payload to the newer draft.
Suggested fix:
// Route the captured payload through SessionChat/useSendMessage so its
// generation-guarded send-error recovery decides whether restoration is safe.
<HappyComposer
onVoiceSend={(text) => props.onSend(text, undefined, null)}
/>
sendMessage: async (_sessionId, text) => {
const accepted = await props.onVoiceSend?.(text)
if (!accepted) throw new Error('Message was not accepted')
}
There was a problem hiding this comment.
Findings
- [Major] Zero-byte standard recordings can overwrite a replacement draft —
stopAndSendclears the captured composer text, butMediaRecorder.stop()queuesonstop. If the user types a new draft before a zero-byte callback runs, this branch restores the old captured text without the generation/current-text guard used by the other failure paths. Evidenceweb/src/hooks/useDictation.ts:125.
Suggested fix:if (pendingSend && !config.getCurrentText().trim()) { config.onTextChange(pendingSend.initialText) }
Summary
Review mode: follow-up after new commits
- One Major draft-corruption path remains in the zero-audio standard-dictation failure branch.
Testing
- Not run (automation): untrusted PR code was not executed.
- Missing coverage: call
stopAndSend, enter replacement text before a zero-byteonstop, then verify the replacement remains unchanged.
HAPI Bot
| saveDraft(pendingSend.sessionId, pendingSend.initialText) | ||
| } | ||
| if (mountedRef.current) { | ||
| if (pendingSend) config.onTextChange(pendingSend.initialText) |
There was a problem hiding this comment.
[MAJOR] Zero-byte recording still overwrites a newer draft. stopAndSend clears the captured text, while MediaRecorder.stop() queues this callback. If the user types replacement text before a zero-byte onstop runs, this unconditional restore replaces it; the new guards cover transcription/send failures but not this branch.
Suggested fix:
if (pendingSend && !config.getCurrentText().trim()) {
config.onTextChange(pendingSend.initialText)
}
There was a problem hiding this comment.
Findings
- [Major] Plain unmounts now upload canceled standard recordings - the unmount cleanup still calls
recorder.stop(), but the rewrittenonstoppath removed the mounted/no-send guard and proceeds totranscribeVoice. Navigating away without requestingstopAndSendtherefore uploads abandoned audio and incurs provider work. Evidenceweb/src/hooks/useDictation.ts:116.
Suggested fix:const pendingSend = sendOnFinishRef.current sendOnFinishRef.current = null if (!mountedRef.current && !pendingSend) return
- [Major] Successful direct voice sends leave the persisted draft stale - both send-on-finish paths call
ApiClient.sendMessagedirectly, bypassing the normaluseSendMessagesuccess handler that runsclearDraftsAfterSend. The composer is empty, but the old sessionStorage draft survives until a later React unmount, so a reload or crash can restore an already-sent message. Evidenceweb/src/hooks/useDictation.ts:146,web/src/hooks/useRealtimeDictation.ts:70.
Suggested fix:await sendMsg(pendingSend.sessionId, finalMessage) clearDraft(pendingSend.sessionId)
- [Minor] Successful realtime stop-and-send leaves the partial transcript visible - the pending-send branch does not call
updatePartial(""), whileHappyComposerrenders any non-emptypartialTranscript. The sent text remains displayed until dictation starts again. Evidenceweb/src/hooks/useRealtimeDictation.ts:65.
Suggested fix:const pendingSend = sendOnFinishRef.current sendOnFinishRef.current = null updatePartial("")
Summary
Review mode: follow-up after new commits
- Two Major lifecycle/persistence regressions and one Minor realtime UI-state regression remain at the latest head.
Testing
- Not run (automation): untrusted PR code was not executed.
- Add coverage for unmount without
stopAndSendnot callingtranscribeVoice, successful standard/realtime sends clearing the persisted draft, and realtime send completion clearingpartialTranscript.
HAPI Bot
| const blob = new Blob(chunksRef.current, { type }) | ||
| recorderRef.current = null | ||
| chunksRef.current = [] | ||
| const pendingSend = sendOnFinishRef.current |
There was a problem hiding this comment.
[MAJOR] Plain unmounts now upload canceled recordings. The cleanup still calls recorder.stop(), and this callback no longer has the original mounted guard. When no explicit stopAndSend is pending, navigating away still reaches transcribeVoice, uploading abandoned audio and incurring provider work.
Suggested fix:
const pendingSend = sendOnFinishRef.current
sendOnFinishRef.current = null
if (!mountedRef.current && !pendingSend) return| if (finalMessage.trim()) { | ||
| const sendMsg = config.sendMessage ?? ((sid: string, msg: string) => config.api!.sendMessage(sid, msg)) | ||
| try { | ||
| await sendMsg(pendingSend.sessionId, finalMessage) |
There was a problem hiding this comment.
[MAJOR] Successful voice sends leave the persisted draft stale. This direct send, and the equivalent realtime branch, bypass the router useSendMessage success handler that calls clearDraftsAfterSend. The composer clears visually, but sessionStorage can retain the already-sent text across a reload or crash.
Suggested fix in both hooks:
await sendMsg(pendingSend.sessionId, finalMessage)
clearDraft(pendingSend.sessionId)| const finish = useCallback(async (text: string) => { | ||
| const pendingSend = sendOnFinishRef.current | ||
| sendOnFinishRef.current = null | ||
| if (pendingSend) { |
There was a problem hiding this comment.
[MINOR] Successful realtime stop-and-send leaves the partial transcript rendered. The pending-send branch never calls updatePartial(""), while HappyComposer renders every non-empty partial. The already-sent text remains visible until the next dictation start.
Suggested fix:
const pendingSend = sendOnFinishRef.current
sendOnFinishRef.current = null
updatePartial("")
There was a problem hiding this comment.
Findings
- [Major] Stopping dictation now sends the transcript automatically — the active stop control routes to
handleDictationToggle, whose changed plain-session branch callsstopAndSend. This contradicts the documented dictation contract and removes the user's chance to inspect or edit recognition errors before the coding agent receives the command. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:504, related contractdocs/guide/voice-assistant.md:5.
Suggested fix:const handleDictationToggle = useCallback( () => dictation.toggle(), [dictation.toggle], ) // Keep stopAndSend in the explicit handleSend path only. if (dictationActive && dictation.status === 'connected' && props.sessionId) { richInputRef.current?.flushSerializedText() const initialText = api.composer().getState().text api.composer().setText('') await dictation.stopAndSend?.(props.sessionId, initialText) return }
Summary
Review mode: follow-up after new commits
- One Major behavior regression remains at the latest head.
Testing
- Not run (automation): untrusted PR code was not executed.
- Add a
HappyComposerinteraction test proving the active Stop control transcribes into the composer without callingsendMessage, while an explicit send usesstopAndSendand remains bound to the original session.
HAPI Bot
| const initialText = api.composer().getState().text | ||
| api.composer().setText('') | ||
| if (targetSessionId) { | ||
| await dictation.stopAndSend?.(targetSessionId, initialText) |
There was a problem hiding this comment.
[MAJOR] Stopping dictation now sends it automatically. The active UnifiedButton routes every connected voice click to onVoiceToggle, so this line makes tapping Stop dispatch the transcript to the coding agent. docs/guide/voice-assistant.md:5 explicitly says stopping dictation inserts text into the composer and never auto-sends it. Recognition errors therefore become unintended commands before the user can inspect or edit them.
Suggested fix:
const handleDictationToggle = useCallback(
() => dictation.toggle(),
[dictation.toggle],
)
// Keep stopAndSend in handleSend / the explicit submit path only.
if (dictationActive && dictation.status === 'connected' && props.sessionId) {
richInputRef.current?.flushSerializedText()
const initialText = api.composer().getState().text
api.composer().setText('')
await dictation.stopAndSend?.(props.sessionId, initialText)
return
}
There was a problem hiding this comment.
Findings
- [Major] Dictation submission bypasses normal composer delivery semantics — the new early branch runs for both
connectedandconnectingbefore attachment, scheduling, scratchlist, queue, and inactive-session handling. It clears the text and calls the text-only voice API, so an attached or scheduled command is sent without its attachments / immediately, and pressing Send during startup can clear the draft even though neither hook has a recorder/session to finish. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:1113, related hook pathsweb/src/hooks/useDictation.ts:195andweb/src/hooks/useRealtimeDictation.ts:247.
Suggested fix:const dictationInFlight = dictationActive && (dictation.status === 'connected' || dictation.status === 'connecting') const canDirectDictationSend = dictation.status === 'connected' && active && attachments.length === 0 && pendingSchedule == null && !props.scratchlistMode && intent === 'default' if (dictationInFlight && !canDirectDictationSend) { await dictation.toggle() return } if (dictationInFlight) { // Existing stopAndSend path. }
- [Major] Late completion can erase a newer target-session draft — success unconditionally clears the session draft and failure unconditionally replaces it. Because this feature deliberately survives navigation,
useComposerDraftcan save a newer draft for the same session before transcription/send settles; these callbacks then delete or overwrite that newer text. The same race exists in realtime mode. Evidenceweb/src/hooks/useDictation.ts:149, relatedweb/src/hooks/useDictation.ts:151,web/src/hooks/useRealtimeDictation.ts:72, and draft persistenceweb/src/hooks/useComposerDraft.ts:183.
Suggested fix:// Capture when stopAndSend starts. const draftAtStart = getDraft(targetSessionId) // Before each late clear/save, preserve any newer non-empty draft. const currentDraft = getDraft(pendingSend.sessionId) const draftUnchanged = currentDraft === '' || currentDraft === pendingSend.draftAtStart if (draftUnchanged) { clearDraft(pendingSend.sessionId) // success // or saveDraft(pendingSend.sessionId, finalMessage) on failure }
Summary
Review mode: follow-up after new commits
- Two Major correctness/data-loss issues remain at the latest reviewed head.
Testing
- Not run (automation): untrusted PR code was not executed.
- Add
HappyComposercoverage for connecting state plus attachments, schedules, scratchlist, and queue intent. - Add standard and realtime hook tests where a newer target-session draft is persisted before the delayed send succeeds or fails.
HAPI Bot
| const resetPendingSendIntent = useCallback(() => { | ||
| if (pendingSendIntentRef) pendingSendIntentRef.current = 'default' | ||
| }, [pendingSendIntentRef]) | ||
|
|
There was a problem hiding this comment.
[MAJOR] This early branch bypasses the normal composer routes. It runs before attachment, scheduling, scratchlist, queue, and inactive-session handling, then clears text and uses a text-only API. An attached/scheduled dictation is therefore sent without attachments / immediately. It also includes connecting; at that point useDictation.stop() and useRealtimeDictation.stop() can have no recorder/session, so no completion callback runs and the cleared draft is lost.
Suggested fix:
const dictationInFlight = dictationActive
&& (dictation.status === 'connected' || dictation.status === 'connecting')
const canDirectDictationSend = dictation.status === 'connected'
&& active
&& attachments.length === 0
&& pendingSchedule == null
&& !props.scratchlistMode
&& intent === 'default'
if (dictationInFlight && !canDirectDictationSend) {
await dictation.toggle()
return
}
if (dictationInFlight) {
// Existing stopAndSend path.
}| const sendMsg = config.sendMessage ?? ((sid: string, msg: string) => config.api!.sendMessage(sid, msg)) | ||
| try { | ||
| await sendMsg(pendingSend.sessionId, finalMessage) | ||
| clearDraft(pendingSend.sessionId) |
There was a problem hiding this comment.
[MAJOR] Late completion can erase a newer session draft. This callback may run after navigation; meanwhile useComposerDraft can persist text newly entered for the target session. Success then clears that newer draft, while the adjacent failure path overwrites it with finalMessage. Realtime mode has the same unconditional mutations at useRealtimeDictation.ts:72-74.
Suggested fix:
// Capture this in stopAndSend.
const draftAtStart = getDraft(targetSessionId)
// Guard every late clear/save.
const currentDraft = getDraft(pendingSend.sessionId)
const draftUnchanged = currentDraft === '' || currentDraft === pendingSend.draftAtStart
if (draftUnchanged) {
clearDraft(pendingSend.sessionId) // or saveDraft(...) on failure
}
There was a problem hiding this comment.
Findings
- [Major] Dictation sends lose Pi steer semantics — the new transcription callback calls
ApiClient.sendMessage(sessionId, text)without a delivery mode. Normal composer sends resolve a fresh default submission tosteerwhile the Pi main session is thinking (web/src/components/SessionChat.tsx:1607), but the hub normalizes a missing mode toqueue(hub/src/sync/messageService.ts:84). Pressing Send during active dictation therefore queues the command behind the current Pi turn instead of steering it. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:473.
Suggested fix:const deliveryMode = resolveMessageDeliveryMode({ agentFlavor, isSessionThinking: thinking, intent: 'default' }) await dictation.stopAndSend(targetSessionId, initialText, deliveryMode) // Carry deliveryMode in pendingSend and forward it as the final // ApiClient.sendMessage(..., deliveryMode) argument.
Summary
Review mode: follow-up after new commits
- One Major correctness issue remains at the latest head.
- The prior attachment/schedule/connecting and late-draft-overwrite findings are resolved by the new commit.
Testing
- Not run (automation): untrusted PR code was not executed.
- Add dictation submission coverage for a thinking Pi session, asserting
deliveryMode: 'steer'; also cover non-thinking/non-Pi queue behavior. - Add realtime
stopAndSendcoverage for unmount, send failure, and newer-draft preservation.
HAPI Bot
| onTextChange: setComposerText, | ||
| sendMessage: async (sessionId: string, text: string) => { | ||
| if (props.voiceTranscriptionApi) { | ||
| await props.voiceTranscriptionApi.sendMessage(sessionId, text) |
There was a problem hiding this comment.
[MAJOR] Direct dictation send drops Pi delivery semantics. This two-argument API call omits deliveryMode. The normal composer resolves a fresh default send to steer when the Pi main session is thinking (SessionChat.tsx:1607-1615), while the hub converts a missing mode to queue (hub/src/sync/messageService.ts:84-93). A dictated command submitted during an active Pi turn will therefore wait behind that turn instead of steering it.
Suggested fix:
const deliveryMode = resolveMessageDeliveryMode({
agentFlavor,
isSessionThinking: thinking,
intent: 'default'
})
await dictation.stopAndSend(targetSessionId, initialText, deliveryMode)
// In the hook callback:
await api.sendMessage(
pendingSend.sessionId,
finalMessage,
undefined,
undefined,
undefined,
pendingSend.deliveryMode,
)
… mention serialization findings (tiann#1435)
…session permission lookup (tiann#1435)
…omposer delivery policy (tiann#1435)
…language and recording support (tiann#1435)
…ment composer draft (tiann#1435)
…ccess, and clear partial transcript (tiann#1435)
… only send button triggers stopAndSend (tiann#1435)
…ecting and protect newer drafts from late overwrite (tiann#1435)
fa88959 to
cf3c918
Compare
There was a problem hiding this comment.
Findings
- [Minor] Dictation send captures stale session activity — the new direct-send gate reads
active, butactiveis absent from thehandleSenddependency list. Session activity can change for the same keyed composer while recording; the callback can then POST directly to a now-inactive session (409) or only stop/transcribe after the session became active instead of sending. Evidenceweb/src/components/AssistantChat/HappyComposer.tsx:1117.
Suggested fix:}, [ api, active, attachments, // ...existing dependencies ])
Summary
Review mode: follow-up after new commits
- One Minor correctness issue remains at the latest head.
- The prior Pi
steerdelivery-mode finding is resolved. - Residual gap: no realtime
stopAndSendcoverage for unmount, send failure, or newer-draft preservation.
Testing
- Not run locally (automation; untrusted PR code was not executed).
- CI
bun typecheckpassed. The latestbun run teststopped on an unrelated timeout atcli/src/claude/claudeRemote.test.ts:72beforetest:web, so the added web tests were not exercised in that run. - Add a
HappyComposerrerender test that flipsactivewhile dictation remains connected, then verifies the current activity state selects direct send versus transcript-only stop.
HAPI Bot
| const handleSend = useCallback(async (intent: ComposerSendIntent = 'default') => { | ||
| if (dictationActive && (dictation.status === 'connected' || dictation.status === 'connecting')) { | ||
| const canDirectSend = dictation.status === 'connected' | ||
| && active |
There was a problem hiding this comment.
[MINOR] handleSend can use a stale active value. The new branch reads active, but the callback dependency list does not include it. Because SessionChat stays keyed to the same session ID when only session.active changes, activity can flip while dictation remains connected without recreating this callback. That can direct-POST to a now-inactive session or fail to direct-send after activation.
Suggested fix:
}, [
api,
active,
attachments,
// ...existing dependencies
])Use shared transfer sampling for standard and realtime sends; update the mounted destination before navigation and unmount. Verified focused voice/transfer tests, typecheck, all five full package suites (CLI rerun serially), and 16 browser checks.
There was a problem hiding this comment.
Findings
- [Major] Same-ID resume failures discard follow-ups saved before resolution —
web/src/hooks/useDictation.ts:296; also realtime dictation. Suggested fix attached inline. - [Major] Successful background transfers erase destination attachment drafts —
web/src/hooks/useDictation.ts:24. Suggested fix attached inline.
Summary
Review mode: follow-up after new commits
Reviewed the full PR diff at 22fbc4cce43a77d0a01fe11ca0188e5fbf3cf18c, using the previous review and compare diff as context. Two remaining draft-loss findings.
Testing
- Not run (automation; PR code was not executed).
- Full PR
git diff --checkpassed. - GitHub
integrationandwindows-codex-mcppassed;testremains in progress. - Missing regressions: same-ID resume failure after saving a follow-up; background success with a saved destination attachment.
HAPI Bot
| // catch compares against this to avoid clobbering text the | ||
| // operator typed into the resumed composer while the request | ||
| // was in flight. | ||
| if (resumed) recoveryDraftAtStart = getDraft(targetSessionId) |
There was a problem hiding this comment.
[Major] Keep the original recovery baseline for same-ID resumes
Start a voice send in inactive session A, type a follow-up while transcription/resume is pending, then navigate away. The composer saves that follow-up under A. If resume returns A itself (supported for in-place resumes), this line replaces recoveryDraftAtStart with the follow-up. When the message POST fails, recoverFailedVoiceSend sees an unchanged persisted draft and excludes it (web/src/hooks/useDictation.ts:86-88); its cross-session source merge also does not apply. Recovery overwrites the follow-up with only the failed voice message.
The same reset occurs in web/src/hooks/useRealtimeDictation.ts:86. Preserve the send-time baseline when the resolved ID is unchanged:
if (resumed && targetSessionId !== pendingSend.sessionId) {
recoveryDraftAtStart = getDraft(targetSessionId)
}Apply in both hooks. Add a regression with a deferred same-ID resolver: save a follow-up and unmount before resolving, reject the POST, then assert the follow-up and failed message both remain in A's draft.
| draftAtStart: string, | ||
| onResolved?: (sessionId: string) => void, | ||
| ): Promise<void> { | ||
| return transferComposerDraftThenNavigate(sourceSessionId, targetSessionId, () => { |
There was a problem hiding this comment.
[Major] Preserve destination attachment drafts during background transfer
While a voice POST from A to resumed session B is pending, open B, attach a file to a follow-up, then leave B. On successful delivery, this new call runs the full attachment transfer even when A has no attachments. buildTransferredAttachments reads only A (web/src/lib/composer-draft-transfer.ts:379-400); moveDraftAttachments replaces B's cache and deletes B's IndexedDB row for an empty source (web/src/lib/composer-attachment-drafts.ts:241,273-274). B's saved file disappears when B is reopened. The text override preserves destination text but does not preserve its attachments. Both dictation modes use this helper.
Suggested fix: add an opt-in preserveTargetAttachments transfer option, enable it for voice transfers, and merge destination attachments into every corrective write. Keep destination upload metadata intact; normalize only source attachments. For example, in transferComposerDraft:
// Load before moving; fail closed on a storage read error.
const targetBase = options?.preserveTargetAttachments
? await loadPersistedAttachments(targetSessionId, { throwOnError: true })
: []
// At the end of buildTransferredAttachments, sample current target edits.
const moved = mergeAttachmentsById(normalizedBase, normalizedPending)
return options?.preserveTargetAttachments
? mergeAttachmentsById(
liveSnapshots.get(targetSessionId)?.attachments ?? targetBase,
moved,
)
: movedAdd a background-success test with a destination attachment persisted before POST completion, then remount B and assert the file survives. Existing new success tests pass empty attachment arrays.
Verified focused regressions, root typecheck, all five complete package suites (environment-sensitive failures rerun serially), and 16 browser checks.
There was a problem hiding this comment.
Findings
- [Major] Failed cross-session voice sends overwrite destination drafts saved before resume resolution —
web/src/hooks/useDictation.ts:86. Evidence and suggested fix attached inline; affects standard and realtime dictation.
Summary
Review mode: follow-up after new commits
Reviewed the full PR diff at 7787a1be7f402367b0c53e30cb79a8cb2e51374e, with the prior review and compare diff as context. The two previous findings are addressed; one remaining draft-loss case is detailed above.
Testing
- Not run (automation; PR code was not executed).
- Full PR
git diff --checkpassed. - GitHub
integrationandwindows-codex-mcppassed;testremains in progress. - Missing regression: failed delivery with a destination draft saved before cross-session resume resolves.
HAPI Bot
| const persistedDraft = getDraft(args.recoverySessionId) | ||
| let replacement = liveReplacement.trim() && liveReplacement !== args.initialText | ||
| ? liveReplacement | ||
| : persistedDraft !== args.draftAtStart ? persistedDraft : '' |
There was a problem hiding this comment.
[Major] Preserve destination drafts saved before resume resolves
When A resumes into B, both hooks snapshot B's persisted draft immediately before the POST (useDictation.ts:296, useRealtimeDictation.ts:86). If the operator has already opened B and saved a follow-up while resume was pending, then leaves B before the POST rejects, persistedDraft === args.draftAtStart here. With neither composer mounted, replacement becomes empty and saveDraft overwrites B's follow-up with only the failed voice message. B's draft was never included in A's submitted text, so it must survive even when unchanged since resolution.
Suggested fix: retain the persisted destination text unconditionally when recovery targets a different session; keep baseline filtering for the original session:
const differentTarget = args.sourceSessionId !== undefined
&& args.sourceSessionId !== args.recoverySessionId
const persistedReplacement = differentTarget
|| persistedDraft !== args.draftAtStart
? persistedDraft
: ''
let replacement = liveReplacement.trim() && liveReplacement !== args.initialText
? liveReplacement
: persistedReplacementAdd standard and realtime regressions with a deferred cross-session resolver: save B's draft before resolving to B, leave both composers unmounted, reject the POST, and assert B retains its draft plus the failed message. Current cross-session failure tests save B's draft after the POST starts, so they do not cover this ordering.
Cross-session recovery always retains destination text and independently merges source follow-ups. Remove destination baseline replacement from both hooks. Verified 72 focused regressions, root typecheck, all five full package suites (Hub rerun serially), and 16 browser checks.
There was a problem hiding this comment.
Findings
- [Major] A newly mounted destination can lose its saved draft when a background voice send fails before hydration —
web/src/hooks/useComposerDraft.ts:55. Evidence and suggested fix are attached inline.
Summary
Review mode: follow-up after new commits
Reviewed the full PR diff at 9379ea537828d9ef08bab759d4edd00037a12b61, using the previous bot review and compare diff as context. The previously reported pre-resolution destination-draft case is addressed; one remaining hydration race is detailed above.
Testing
- Not run (automation; PR code was not executed).
- Full PR
git diff --checkpassed. - Missing regression: standard/realtime failure recovery after destination mount but before its animation-frame draft restore.
HAPI Bot
| useEffect(() => { | ||
| if (!sessionId) return | ||
| const composer = { | ||
| getText: () => composerTextRef.current, |
There was a problem hiding this comment.
[Major] Preserve persisted text until the registered composer is hydrated
The registry exposes getText() as '' immediately after mount, but the saved draft is restored later in requestAnimationFrame (useComposerDraft.ts:99). If A's pending send resumes into B, the operator opens B with an existing saved draft, and the POST rejects before that frame, recoverFailedVoiceSend uses B's empty registered text instead of its persisted draft (useDictation.ts:88). It then overwrites storage and the composer with the failed message (useDictation.ts:95). The subsequent hydration reads the overwritten value, so B's original text is lost. Both standard and realtime sends use this recovery helper.
Suggested fix: make the registered getter preserve the persisted value before text hydration, and update the ref synchronously when hydration applies text. Keep an intentional empty composer authoritative after hydration:
// In the registered composer:
getText: () => draftReadyRef.current
? composerTextRef.current
: (composerTextRef.current || getDraft(sessionId)),
// In the existing restoreText block, before marking draftReadyRef:
composerTextRef.current = draft
setText(draft)Add standard/realtime regressions that hold the destination's animation-frame callback, reject the pending send after mounting that destination, then flush hydration and unmount. Assert the original destination draft plus the failed message survives. Current voice regressions wait for the destination text to hydrate before rejecting, so they miss this ordering.
Expose persisted draft text until hydration and update the text ref synchronously when restoring. Cover both voice modes with a held animation frame and intentional empty text after hydration. Verified 94 focused checks, full package suites, root typecheck, and 16 browser checks.
There was a problem hiding this comment.
Findings
No reportable issues found in the full PR diff.
Summary
Review mode: follow-up after new commits
Reviewed all 19 changed files at 0a7bbc3b81d44050a53c8aca788fd074b12f5c57, using the previous bot review and compare diff as context. The previously reported destination-text hydration race is addressed. Residual risk: the realtime tests mock ElevenLabs and do not exercise provider teardown when navigation occurs before the final transcript arrives.
Testing
- Not run (automation; PR code was not executed).
- Full PR merge-base
git diff --checkpassed. - Suggested coverage: unmount with realtime finalization pending, then deliver the final transcript; verify exactly one POST to the original/resolved session and transport cleanup.
HAPI Bot
…ice-input-session-switch
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicitly requested voice sends should complete for their original session after navigation. This addresses an existing gap in dictation lifecycle handling.
Evidence
- Issue #1435 specifies sending from Session A, navigating away, and completing delivery to Session A.
- The base dictation hooks discard transcription results or cancel provider sessions on unmount.
Approach — Pass
Capturing the target session, preserving pending finalization, resolving inactive sessions before delivery, and recovering drafts is a viable approach.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1201 captures the session and resume callbacks for stopAndSend.
- web/src/hooks/useDictation.ts:290 resolves the target before posting; shared helpers handle draft transfer and recovery.
- web/src/router.tsx:591 checks the current route before navigating after background completion.
Code — Reviewed
Reviewed all 18 changed files at 66772e6. Found one draft-recovery defect. Realtime provider teardown remains insufficiently covered.
- [Minor] Respect an intentionally empty live draft during recovery —
web/src/hooks/useDictation.ts:84
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed. Merge-base git diff --check passed.
- Fixed-head CI: integration and windows-codex-mcp passed; test remained in progress when checked.
- Added realtime tests mock ElevenLabs and finalize transcription before unmount. Coverage is still missing for unmount during pending finalization, followed by a final transcript, exactly one delivery, and transport cleanup.
HAPI Bot
| ? args.sourceSessionId | ||
| : undefined | ||
| const persistedDraft = getDraft(args.recoverySessionId) | ||
| let replacement = liveReplacement.trim() && liveReplacement !== args.initialText |
There was a problem hiding this comment.
[Minor] Respect an intentionally empty live draft during recovery
Start a voice send in active Session A, type a follow-up while delivery is pending, navigate away and back, then delete the restored follow-up. If the original send subsequently fails, liveReplacement is empty but persistedDraft still contains the follow-up: active composers persist text on unmount/page hiding, not each edit. This fallback restores the deliberately deleted text alongside the failed message. Both dictation modes use this helper.
Suggested fix
When a live composer is available, treat its current text—including an empty string—as authoritative. Consult persisted text only when no live composer is available. Add regression coverage for clearing a restored follow-up before a background send rejects.
…oss-session delivery
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should finish for its original session after navigation. This addresses an existing dictation lifecycle gap.
Evidence
- Issue #1435 specifies initiating a voice send in Session A, navigating away, and completing delivery to Session A.
- Base web/src/hooks/useDictation.ts:116 discards transcription results after unmount; base useRealtimeDictation cancels provider sessions during unmount.
Approach — Pass
Capturing the target, preserving pending finalization, resolving inactive sessions before delivery, and merging follow-up drafts is viable. The remaining provider lifecycle defect can be fixed within this direction.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1204 captures the target and resolution callbacks.
- web/src/hooks/useDictation.ts:290 resolves the session before posting; shared helpers transfer and recover drafts.
- web/src/router.tsx:591 reads the current route before navigating after background completion.
Code — Reviewed
Reviewed the entire 18-file merge-base diff at 03316fe. The previous empty-draft recovery defect is fixed; one ElevenLabs finalization defect remains.
- [Major] Preserve the ElevenLabs connection during pending finalization —
web/src/hooks/useRealtimeDictation.ts:327
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, and windows-codex-mcp succeeded.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
- The SDK unmount behavior was verified against the locked 0.13.0 release source through the GitHub API; live provider finalization was not exercised.
HAPI Bot
| mountedRef.current = true | ||
| return () => { | ||
| mountedRef.current = false | ||
| if (sendOnFinishRef.current) return |
There was a problem hiding this comment.
[Major] Preserve the ElevenLabs connection during pending finalization
Press Send during ElevenLabs dictation, then navigate before the committed transcript arrives. This early return skips only HAPI's cleanup: the locked @elevenlabs/react 0.13.0 useScribe hook independently calls connectionRef.current?.close() on unmount (packages/react/src/scribe.ts:200–205). That terminates transcription despite the pending send. HAPI subsequently falls back to partialRef.current at line 252, potentially sending truncated text or sending nothing when no partial transcript has arrived. The added background tests emit the committed transcript synchronously before unmount and mock away SDK cleanup.
Suggested fix
Own the ElevenLabs connection outside the session component's unmount lifecycle, using a persistent owner or the client SDK with explicit cleanup. Close it after finalization completes. Add coverage that unmounts while commit is pending, then delivers the final transcript and verifies exactly one complete send and transport cleanup.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should complete for its original session after navigation. This addresses a verified dictation lifecycle gap and prevents losing the requested message.
Evidence
- Issue #1435 specifies sending from Session A, navigating away, and completing delivery to Session A.
- Base web/src/hooks/useDictation.ts:116 discards transcription results after unmount; base realtime dictation cancels its provider session during cleanup.
Approach — Pass
Capturing the target and send options, preserving pending finalization, resolving inactive sessions before delivery, and merging follow-up drafts addresses the requirement. Explicit ownership of the ElevenLabs connection fixes the previous SDK unmount problem.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1204 captures the target and resolution callbacks.
- web/src/hooks/useDictation.ts:290 resolves the session before posting; shared transfer and recovery helpers preserve follow-up text.
- web/src/hooks/useRealtimeDictation.ts:193 owns the client connection; its stop handler waits for a committed transcript or bounded fallback before closing.
- web/src/router.tsx:591 checks the current route before navigating after background completion.
Code — Reviewed
Reviewed the entire 20-file merge-base diff at 3cc695e. No actionable defects were established. The previous ElevenLabs finalization finding is addressed. Assessment is based on static inspection; live provider behavior remains unverified.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, windows-codex-mcp, and image succeeded.
- Added regression coverage includes delayed ElevenLabs commit after unmount, timeout fallback, ordinary cancellation, resumed-session drafts, and failure recovery.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
- Connection lifecycle semantics were checked against the locked ElevenLabs client 0.13.0 source through the GitHub API; live transcription-provider finalization was not exercised.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should finish for its original session after navigation. This addresses a verified dictation lifecycle gap.
Evidence
- Issue #1435 specifies background transcription and delivery to Session A after navigating away.
- Base web/src/hooks/useDictation.ts:116 discards transcription results after unmount; base realtime cleanup cancels the provider session.
Approach — Pass
Capturing the target and send options, retaining pending finalization, resolving inactive sessions, and preserving follow-up drafts is a viable approach.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1208 captures the session and resolution callbacks.
- web/src/hooks/useDictation.ts:296 resolves the target before delivery; web/src/hooks/useRealtimeDictation.ts:328 preserves pending finalization across unmount.
- web/src/router.tsx:592 guards navigation using the current route.
Code — Reviewed
Reviewed the entire 22-file merge-base diff at the fixed head. One composer integration defect was identified through static inspection.
- [Minor] Refresh direct-send eligibility after draft hydration —
web/src/components/AssistantChat/HappyComposer.tsx:1192
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, windows-codex-mcp, and image succeeded.
- Added regression tests cover navigation, resumed-session drafts, repeated follow-up text, failure recovery, and delayed ElevenLabs finalization; they do not cover the composer hydration transition described above.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
- Live transcription-provider finalization remains unverified.
HAPI Bot
| && dictationDirectSendEligible({ | ||
| active, | ||
| resolveSessionIdAvailable: props.resolveSessionIdForVoice !== undefined, | ||
| blocksScheduling, |
There was a problem hiding this comment.
[Minor] Refresh direct-send eligibility after draft hydration
handleSend reads blocksScheduling but omits it from its useCallback dependencies. If dictation connects on an inactive, empty session before attachment hydration finishes, resolving hydration with no stored attachments changes blocksScheduling from true to false without changing the declared dependencies. The visible Send button becomes eligible, but its callback retains the earlier blocker and only stops/transcribes. Navigating away then drops the transcript instead of completing the requested send.
Suggested fix
Add blocksScheduling to handleSend's dependency array. Add a composer integration regression that delays empty attachment hydration until recording is connected, then clicks Send and verifies stopAndSend is invoked.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should complete for its original session after navigation. Existing dictation cleanup prevents this.
Evidence
- Issue #1435 specifies background transcription and delivery to Session A after navigating away.
- Base web/src/hooks/useDictation.ts:116 discards transcription after unmount; base web/src/hooks/useRealtimeDictation.ts:240 disconnects realtime transcription.
Approach — Pass
Capturing the destination and send intent, retaining pending finalization across unmount, and preserving follow-up drafts is a viable direction. The remaining resolver defect can be fixed within this approach.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1208 captures the session and resolution callbacks.
- web/src/hooks/useRealtimeDictation.ts:328 preserves pending finalization across unmount.
- web/src/router.tsx:592 checks the current route before navigating after background completion.
Code — Reviewed
Reviewed the entire 23-file merge-base diff at the fixed head. The previous hydration finding is addressed. One remaining background-delivery defect was identified through static inspection.
- [Minor] Refresh session activity before deferred voice delivery —
web/src/router.tsx:828
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, windows-codex-mcp, and image succeeded.
- Added coverage exercises navigation, draft preservation and recovery, delayed ElevenLabs finalization, and attachment hydration. Session resolvers are mocked in the hook tests, leaving the reported activity transition uncovered.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
- Live transcription-provider finalization remains unverified.
HAPI Bot
| onSend={sendMessage} | ||
| resolveSessionIdForUpload={async (id) => (await resolveSessionId(id)).sessionId} | ||
| onUploadSessionResolved={handleSessionResolved} | ||
| resolveSessionIdForVoice={resolveSessionId} |
There was a problem hiding this comment.
[Minor] Refresh session activity before deferred voice delivery
The supplied resolver closes over session.active from its render (router.tsx:517–519), and stopAndSend retains that callback throughout transcription. If Session A is active when Send is clicked but its CLI disconnects while transcription is pending, the retained resolver still returns resumed:false. The subsequent message POST receives 409 session_inactive, so the requested background send becomes a recovered draft instead of completing. Both dictation modes retain this resolver.
Suggested fix
Provide a voice resolver that reads the target session's current activity at finalization time and resumes it when necessary. Add a regression covering an active-to-inactive transition during deferred transcription using the actual resolver behavior.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should finish for its original session after navigation. This addresses an existing loss of pending transcription and fits HAPI’s remote-control workflow.
Evidence
- Issue #1435 explicitly requests background transcription and delivery to Session A after navigating away.
- Base web/src/hooks/useDictation.ts:116 discards transcription after unmount; base web/src/hooks/useRealtimeDictation.ts:240 disconnects realtime transcription.
Approach — Pass
Capturing send intent and destination, retaining finalization across unmount, refreshing target activity before delivery, and preserving follow-up drafts provide a viable solution.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1208 captures the destination and resolution callbacks.
- web/src/hooks/useRealtimeDictation.ts:328 retains explicitly requested finalization across unmount.
- web/src/lib/dictationSend.ts:44 reads current session activity and permission settings before resuming.
- web/src/router.tsx:597 checks the current route before navigating after background completion.
Code — Reviewed
Reviewed the entire 23-file merge-base diff at 03a2bcc. The previous stale-activity finding is addressed. No additional actionable defects were identified through static inspection.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, windows-codex-mcp, and image succeeded.
- Added regression coverage exercises background finalization, draft preservation and recovery, attachment hydration, and activity transitions using the production voice resolver.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
- Live transcription-provider finalization remains unverified.
HAPI Bot
Voice dictation, realtime dictation and the cross-session voice tool sent through api.sendMessage with localId null. Shared Codex then minted a random clientUserMessageId for the turn and echoed the user message back with it; the hub dedupes only on (session, localId), so every voice message was stored and shown twice. Route all voice sends through sendVoiceMessage, which mints the same local-* id useSendMessage uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
An explicitly requested voice send should complete for its original session after navigation. The existing hooks cancel or discard pending transcription, so this addresses a supported use case.
Evidence
- Issue #1435 explicitly requests background transcription and delivery to Session A after navigating away.
- Base web/src/hooks/useDictation.ts:116 discards transcription after unmount; base web/src/hooks/useRealtimeDictation.ts:240 disconnects realtime transcription.
Approach — Pass
Capturing send intent and destination, retaining requested finalization across unmount, resolving current session activity before delivery, and preserving drafts provide a viable solution. Client-generated message IDs follow the existing deduplication pattern.
Evidence
- web/src/components/AssistantChat/HappyComposer.tsx:1208 captures the target and session-resolution callbacks.
- web/src/hooks/useRealtimeDictation.ts:329 retains explicitly requested finalization across unmount.
- web/src/lib/dictationSend.ts:59 fetches current activity and permissions before resuming.
- web/src/router.tsx:578 checks the live route before navigating after completion.
- web/src/lib/dictationSend.ts:17 supplies a distinct localId for voice sends.
Code — Reviewed
Reviewed the entire 23-file merge-base diff at 45bf446. No actionable defects were established through static inspection. Live transcription-provider behavior remains unverified.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- Fixed-head GitHub checks test, integration, windows-codex-mcp, and image succeeded.
- Added regression coverage exercises background finalization, draft recovery and preservation, current session activity, attachment hydration, and distinct voice-message IDs.
- Merge-base git diff --check passed. No PR code, tests, builds, or scripts were executed.
HAPI Bot
This branch has not been deployed
2 participants
Closes #1435.
Result
An explicitly requested voice send finishes for its original session even if the operator navigates away while transcription or delivery is pending. The voice resolver fetches current session activity and permission settings when transcription finishes, resumes inactive targets before delivery, and avoids reusing render-time activity. Background completion respects the operator's current page.
Standard and realtime sends preserve source and destination follow-up drafts on success and failure, including new text identical to the submitted draft. Session-scoped edit revisions distinguish a fresh follow-up from an unchanged persisted baseline; resumed transfers sample live composer text when persistence completes. Failed delivery restores the retryable transcript without reviving intentionally deleted text.
Realtime finalization owns the already-locked ElevenLabs Scribe client connection until a committed transcript, bounded fallback, or error. Ordinary recording cancellation still closes it. The current upstream composer layout retains both the large Stop control and the compact voice Send path, alongside existing rich-input flushing, attachments, scheduling, and delivery-mode guards.
Voice sends carry a client
localId, like ordinary sends. Without one, shared Codex assigned the turn a randomclientUserMessageIdand echoed the user message back under it; the hub could not dedupe the echo, so every voice message appeared twice in Codex sessions.The direct-send callback also refreshes when empty attachment hydration completes, so its visible eligibility and the invoked recording action remain consistent.
Validation — 2026-10-18
f13a1aaea. Root typecheck and the complete Web suite passed: 3,439 tests.sendVoiceMessageregression coverage asserts a distinctlocal-*id per send; dictation tests now require a localId instead ofnull. Production evidence before the fix: each dictated Codex message was stored as awebapprow withlocalId: nullfollowed by an identicalcliecho.Validation — 2026-10-14
5847b034einto the existing branch while retaining its previous head as an ancestor.Live transcription-provider finalization remains unverified.
AI disclosure: Codex assisted with integration, regression tests, and verification.